Don't recompile nested deps too frequently

When compiling a package with a nested dependency, any modification to the outer
package would trigger a recompilation of the inner package. This commit alters
the fingerprint() method to take a PackageId to query about the location of a
package and only lookup the files relevant to that package.

The dependency structure of a PathSource is now everything rooted at the
original Cargo.toml minus all subdirectories which contain a Cargo.toml

diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs
index 32284f82f8972db00b7eb1542868860ccb2a6352..b9cc31921ad5489e69d3ffa8aa4601eeff278879 100644 (file)
--- a/src/cargo/core/package.rs
+++ b/src/cargo/core/package.rs
@@ -128,7 +128,7 @@ impl Package {
         let sources = sources.iter().map(|source_id| {
             source_id.load(config)
         }).collect::<Vec<_>>();
-        SourceSet::new(sources).fingerprint()
+        SourceSet::new(sources).fingerprint(self)
     }
 }
 

diff --git a/src/cargo/core/source.rs b/src/cargo/core/source.rs
index 3214a8eb0588373add1b1309678d4d2ced0f5e77..6e7192a359a1c79c63f98771e01f1907f7c1cd85 100644 (file)
--- a/src/cargo/core/source.rs
+++ b/src/cargo/core/source.rs
@@ -39,7 +39,10 @@ pub trait Source {
     /// This fingerprint is used to determine the "fresheness" of the source
     /// later on. It must be guaranteed that the fingerprint of a source is
     /// constant if and only if the output product will remain constant.
-    fn fingerprint(&self) -> CargoResult<String>;
+    ///
+    /// The `pkg` argument is the package which this fingerprint should only be
+    /// interested in for when this source may contain multiple packages.
+    fn fingerprint(&self, pkg: &Package) -> CargoResult<String>;
 }
 
 #[deriving(Show, Clone, PartialEq, Eq, PartialOrd, Ord)]
@@ -203,10 +206,10 @@ impl Source for SourceSet {
         Ok(ret)
     }
 
-    fn fingerprint(&self) -> CargoResult<String> {
+    fn fingerprint(&self, id: &Package) -> CargoResult<String> {
         let mut ret = String::new();
         for source in self.sources.iter() {
-            ret.push_str(try!(source.fingerprint()).as_slice());
+            ret.push_str(try!(source.fingerprint(id)).as_slice());
         }
         return Ok(ret);
     }

diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs
index 1868023c04ed17ccec3fba6b159c898cc84231ff..97462af20852a75cc20039f2fa142023b9378024 100644 (file)
--- a/src/cargo/sources/git/source.rs
+++ b/src/cargo/sources/git/source.rs
@@ -133,7 +133,7 @@ impl<'a, 'b> Source for GitSource<'a, 'b> {
         self.path_source.get(ids)
     }
 
-    fn fingerprint(&self) -> CargoResult<String> {
+    fn fingerprint(&self, _pkg: &Package) -> CargoResult<String> {
         let db = self.remote.db_at(&self.db_path);
         db.rev_for(self.reference.as_slice())
     }

diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs
index 225b35c0247da3d556778248c5bcbe2744f12862..26baccd57201ef0cc701233f4e3cd423cc0a2eaf 100644 (file)
--- a/src/cargo/sources/path.rs
+++ b/src/cargo/sources/path.rs
@@ -48,6 +48,14 @@ impl PathSource {
             None => Err(internal("no package found in source"))
         }
     }
+
+    fn read_packages(&self) -> CargoResult<Vec<Package>> {
+        if self.updated {
+            Ok(self.packages.clone())
+        } else {
+            ops::read_packages(&self.path, &self.id)
+        }
+    }
 }
 
 impl Show for PathSource {
@@ -59,9 +67,9 @@ impl Show for PathSource {
 impl Source for PathSource {
     fn update(&mut self) -> CargoResult<()> {
         if !self.updated {
-          let pkgs = try!(ops::read_packages(&self.path, &self.id));
-          self.packages.push_all_move(pkgs);
-          self.updated = true;
+            let packages = try!(self.read_packages());
+            self.packages.push_all_move(packages);
+            self.updated = true;
         }
 
         Ok(())
@@ -87,17 +95,28 @@ impl Source for PathSource {
            .collect())
     }
 
-    fn fingerprint(&self) -> CargoResult<String> {
-        let mut max = None;
-        let target_dir = self.path.join("target");
-        for child in try!(fs::walk_dir(&self.path)) {
-            if target_dir.is_ancestor_of(&child) { continue }
-            let stat = try!(fs::stat(&child));
-            max = cmp::max(max, Some(stat.modified));
+    fn fingerprint(&self, pkg: &Package) -> CargoResult<String> {
+        let packages = try!(self.read_packages());
+        let mut max = 0;
+        for pkg in packages.iter().filter(|p| *p == pkg) {
+            let loc = pkg.get_manifest_path().dir_path();
+            max = cmp::max(max, try!(walk(&loc, true)));
         }
-        match max {
-            None => Ok(String::new()),
-            Some(time) => Ok(time.to_str()),
+        return Ok(max.to_str());
+
+        fn walk(path: &Path, is_root: bool) -> CargoResult<u64> {
+            if !path.is_dir() {
+                return Ok(try!(fs::stat(path)).modified)
+            }
+            // Don't recurse into any sub-packages that we have
+            if !is_root && path.join("Cargo.toml").exists() { return Ok(0) }
+
+            let mut max = 0;
+            for dir in try!(fs::readdir(path)).iter() {
+                if is_root && dir.filename_str() == Some("target") { continue }
+                max = cmp::max(max, try!(walk(dir, false)));
+            }
+            return Ok(max)
         }
     }
 }

diff --git a/tests/test_cargo_compile_path_deps.rs b/tests/test_cargo_compile_path_deps.rs
index f3063d678cb4b593c76f934a95dedba41679e37e..635808eb16564a938fe0219e5a5642db87129602 100644 (file)
--- a/tests/test_cargo_compile_path_deps.rs
+++ b/tests/test_cargo_compile_path_deps.rs
@@ -303,3 +303,56 @@ test!(no_rebuild_two_deps {
                                             FRESH, bar.display(),
                                             FRESH, p.root().display())));
 })
+
+test!(nested_deps_recompile {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+
+            name = "foo"
+            version = "0.5.0"
+            authors = ["wycats@example.com"]
+
+            [dependencies.bar]
+
+            version = "0.5.0"
+            path = "src/bar"
+
+            [[bin]]
+
+            name = "foo"
+        "#)
+        .file("src/foo.rs",
+              main_file(r#""{}", bar::gimme()"#, ["bar"]).as_slice())
+        .file("src/bar/Cargo.toml", r#"
+            [project]
+
+            name = "bar"
+            version = "0.5.0"
+            authors = ["wycats@example.com"]
+
+            [[lib]]
+
+            name = "bar"
+        "#)
+        .file("src/bar/src/bar.rs", "pub fn gimme() {}");
+    let bar = p.root();
+
+    assert_that(p.cargo_process("cargo-build"),
+                execs().with_stdout(format!("{} bar v0.5.0 (file:{})\n\
+                                             {} foo v0.5.0 (file:{})\n",
+                                            COMPILING, bar.display(),
+                                            COMPILING, p.root().display())));
+    // See comments for the above `sleep`
+    timer::sleep(1000);
+    File::create(&p.root().join("src/foo.rs")).write_str(r#"
+        fn main() {}
+    "#).assert();
+
+    // This shouldn't recompile `bar`
+    assert_that(p.process("cargo-build"),
+                execs().with_stdout(format!("{} bar v0.5.0 (file:{})\n\
+                                             {} foo v0.5.0 (file:{})\n",
+                                            FRESH, bar.display(),
+                                            COMPILING, p.root().display())));
+})